Skip to content

[Fix] Copy commandFactory when creating DatabricksClient for unified provider#5503

Merged
Divyansh-db merged 1 commit intomainfrom
divyansh-vijayvergia_data/fix-DatabricksClientForUnifiedProvider
Mar 20, 2026
Merged

[Fix] Copy commandFactory when creating DatabricksClient for unified provider#5503
Divyansh-db merged 1 commit intomainfrom
divyansh-vijayvergia_data/fix-DatabricksClientForUnifiedProvider

Conversation

@Divyansh-db
Copy link
Copy Markdown
Contributor

@Divyansh-db Divyansh-db commented Mar 20, 2026

Changes

Fix getDatabricksClientForUnifiedProvider not copying commandFactory

Summary

  • getDatabricksClientForUnifiedProvider creates a new DatabricksClient when routing to a specific workspace, but only copies the inner
    *client.DatabricksClient — it does not copy commandFactory, causing a nil pointer panic when resources call CommandExecutor()
  • This affects databricks_mount, which executes Python on clusters via the Commands API and is the only resource using CommandExecutor through DatabricksClientForUnifiedProvider
  • The fix copies commandFactory from the parent client, consistent with the existing pattern in ClientForHost (common/client.go:604)

Root Cause

getDatabricksClientForUnifiedProvider constructs the returned client as:

return &DatabricksClient{
DatabricksClient: client,
// commandFactory is missing!
}, nil

CommandExecutor() calls c.commandFactory(ctx, c), which panics when commandFactory is nil.

This bug was latent on main because resources using DatabricksClientForUnifiedProvider with a workspace-level provider always got workspaceID == "" from
state, short-circuiting to return the original client (which has commandFactory). The bug is only triggered when provider_config { workspace_id } is
explicitly set, routing through getDatabricksClientForUnifiedProvider.

Test plan

  • Unit test: TestGetDatabricksClientForUnifiedProvider_CopiesCommandFactory — verifies CommandExecutor works on the returned client (both direct and via public API)
  • Acceptance test: TestAccCreateDatabricksMountWithProviderConfig — creates a mount with provider_config { workspace_id } on a workspace-level provider, exercising the full code path end-to-end
  • Verified without fix: cannot create mount: panic: runtime error: invalid memory address or nil pointer dereference
  • Verified with fix: test passes
  • All existing common/ unit tests pass

NO_CHANGELOG=true

@Divyansh-db Divyansh-db requested review from a team as code owners March 20, 2026 13:37
@Divyansh-db Divyansh-db requested review from tanmay-db and removed request for a team March 20, 2026 13:37
@github-actions
Copy link
Copy Markdown

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

  • PR number: 5503
  • Commit SHA: 6b7f29e746e838c9e4e55874b1b263c6c378803f

Checks will be approved automatically on success.

@Divyansh-db Divyansh-db changed the title fixed broken code path for DatabricksClientForUnifiedProvider Fix broken code path for DatabricksClientForUnifiedProvider Mar 20, 2026
@Divyansh-db Divyansh-db changed the title Fix broken code path for DatabricksClientForUnifiedProvider [Fix] Copy commandFactory when creating DatabricksClient for unified provider Mar 20, 2026
@Divyansh-db Divyansh-db added this pull request to the merge queue Mar 20, 2026
Merged via the queue into main with commit c4171e5 Mar 20, 2026
17 of 18 checks passed
@Divyansh-db Divyansh-db deleted the divyansh-vijayvergia_data/fix-DatabricksClientForUnifiedProvider branch March 20, 2026 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants